feat(amazonq): Adding openDiff and acceptDiff UX to the agentic chat. #6846

laileni-aws · 2025-03-25T00:50:35Z

Problem

Agentic chat does not view the generated code diff and no option for accept or reject the code diff
Splitting this bulk PR: feat(amazonq): Agentic chat UX POC #6843

Solution

Adding UX improvements and agent loop mechanism to Q chat.

QChatPOCFileWrite.mov

TODO:

Metrics
Refactoring code
More UX improvements
Pending part 2 of feat(amazonq): Agentic chat UX POC #6843

Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
Your code changes must meet the guidelines in CONTRIBUTING.md.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

github-actions · 2025-03-25T00:50:48Z

This pull request modifies code in src/* but no tests were added/updated.
- Confirm whether tests should be added or ensure the PR description explains why tests are not required.
This pull request implements a feat or fix, so it must include a changelog entry (unless the fix is for an unreleased feature). Review the changelog guidelines.
- Note: beta or "experiment" features that have active users should announce fixes in the changelog.
- If this is not a feature or fix, use an appropriate type from the title guidelines. For example, telemetry-only changes should use the telemetry type.

ctlai95 · 2025-03-25T19:47:03Z

Can you please update the title to be more specific to this change?

ctlai95 · 2025-03-25T19:53:44Z

packages/core/src/amazonq/webview/ui/apps/cwChatConnector.ts

+        if (!this.onChatAnswerUpdated || !['accept-code-diff', 'reject-code-diff'].includes(action.id)) {
+            return
+        }
+        const answer: ChatItem = {
+            type: ChatItemType.ANSWER,
+            messageId: messageId,
+            buttons: [],
+        }
+        switch (action.id) {
+            case 'accept-code-diff':
+                answer.buttons = [
+                    {
+                        keepCardAfterClick: true,
+                        text: 'Accepted code',
+                        id: 'accepted-code-diff',
+                        status: 'success',
+                        position: 'outside',
+                        disabled: true,
+                    },
+                ]
+                break
+            case 'reject-code-diff':
+                answer.buttons = [
+                    {
+                        keepCardAfterClick: true,
+                        text: 'Rejected code',
+                        id: 'rejected-code-diff',
+                        status: 'error',
+                        position: 'outside',
+                        disabled: true,
+                    },
+                ]
+                break
+            default:
+                break
+        }
+        this.onChatAnswerUpdated(tabId, answer)


I feel like this shouldn't live in this function. Is there somewhere else we can move this to?

This is existing way how the buttons work/setup in both VSC and JB.
We can create a function and return answer but basically the functionality does not change.

But this logic is specific to diffs which I don't think belongs here. Why not create a separate diff clicked handler? I see we already have some custom logic in

aws-toolkit-vscode/packages/core/src/amazonq/webview/ui/connector.ts

Line 707 in 011654d

if (action.id === `open-settings`) {

can't we just add to that?

This logic is not specific to diff's we can extend this to feedback button and even for Confirm button!
Here we show a UX to disable the button once user click on this button.

ctlai95 · 2025-03-25T19:57:26Z

packages/core/src/amazonq/webview/ui/main.ts

                        fileTreeTitle: '',
                        filePaths: item.contextList.map((file) => file.relativeFilePath),
-                        rootFolderTitle: 'Context',
+                        rootFolderTitle: item.title,


Maybe fallback to ?? 'Context'

Context is not a generic text across Q so, I feel this should be either undefined or we can supply proper text as part of title which we do in our changes.

Got it, the reason I'm suggesting this is because this code is shared with all agents. So unless we are sure that we always pass "Context" in the title for all messages everywhere, there might be a UI regression.

Got it, right now only Q chat uses Context or contextList in project falcon but going forward we need to pass title from the agent controller.ts instead of static text.

ctlai95 · 2025-03-25T19:59:29Z

packages/core/src/amazonq/webview/ui/apps/cwChatConnector.ts

    onFileClick = (tabID: string, filePath: string, messageId?: string) => {
        this.sendMessageToExtension({
-            command: 'file-click',
+            command: messageId === '' ? 'file-click' : 'open-diff',


This feels a bit hacky. Can there be a separate onOpenDiff handler?

There used to be onOpenDiff function previously but this got deprecated

@deprecated since version 4.6.3. Will be dropped after version 5.x.x. Use {@link onFileClick} instead

So Right now if user click on any file within fileTreeDiff Mynah sends event through onFileClick.

I think we need to think about this more. It's not clear to me why if messageId is empty that it should be file-click and not open-diff

So, basically the question is, How can we differentiate the ContextList vs FileListTree ?

ctlai95 · 2025-03-25T20:00:47Z

packages/core/src/amazonq/webview/ui/apps/cwChatConnector.ts


    onCustomFormAction(
        tabId: string,
+        messageId: string,


Should it be string | undefined? Then you don't need to fallback to ''

This wont be undefined and this is for onCustomFormAction not for onFileClick. Can you elaborate more on this?

Sorry I didn't understand, aren't you passing in an empty string in connector.ts?

this.cwChatConnector.onCustomFormAction(tabId, messageId ?? '', action)

Ohh I got it,
I am passing '' as part of messageId because of this code

I tested passing undefined and got undesirable result. So, I am passing ''
I remember testing this for RIV for /test too but I will give a shot once again after UX integration.

ctlai95 · 2025-03-25T20:05:18Z

packages/core/src/codewhispererChat/view/connector/connector.ts

    }
 }

+export interface FileList {


Can't we use the type from ChatItemContent?

Yeah actually we can use ChatItemContent['fileList']

ctlai95 · 2025-03-25T20:08:10Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts

+        if (filePath) {
+            this._listOfReadFiles.push(filePath)
+        } else {
+            this._listOfReadFiles = []


Why does this reset the list? Looks like it would behave unexpectedly if I just look at the name of the function and not the implementation

If user tries to ask any other question in between generation or if user gets any error we need to reset to files read
To show proper UX of
2 files read, reading Sample.java
3 files read, reading foo.java

etc..

Sure but that shouldn't happen here. Someone reading the code in the future is not going to know why it's being reset

Let me write a separate method(clearListOfReadFiles()) to reset the files.

ctlai95 · 2025-03-25T20:11:11Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts

+    public get listOfReadFiles(): string[] {
+        return this._listOfReadFiles
+    }
+    public get getFilePath(): string | undefined {


This is already a getter function, get in the name is redundant

Suggested change

public get getFilePath(): string | undefined {

public get filePath(): string | undefined {

ctlai95 · 2025-03-25T20:13:43Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts


 export class ChatSession {
    private sessionId?: string
+    private _listOfReadFiles: string[] = []


list is redundant

Required for the Reading files UX which still a TODO. I can revisit on this once we have final UX into place.

I meant that the variable name can just be _readFiles

ctlai95 · 2025-03-25T20:14:47Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts

+    private _listOfReadFiles: string[] = []
+    private _filePath: string | undefined
+    private _tempFilePath: string | undefined


Please check the naming or add some documentation, it's very unclear what these are used for

I will add some comments on top of this variable. There might be some duplication due to lot of dependencies on our PR's.
I will add a TODO to revisit and remove necessary variables if required.

ctlai95 · 2025-03-25T20:19:32Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts

        this.sessionId = id
    }
+    public get listOfReadFiles(): string[] {
+        return this._listOfReadFiles


Actually I think we can get this information from chatHistory?

Please can you point me the code where this was implemented? I checked the latest PR and looks like its still TODO

ctlai95 · 2025-03-25T21:51:01Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts

    public setSessionID(id?: string) {
        this.sessionId = id
    }
+    public get listOfReadFiles(): string[] {


Suggested change

public get listOfReadFiles(): string[] {

public get readFiles(): string[] {

mr-lee

Approved to unblock.

mr-lee · 2025-03-25T21:57:38Z

packages/core/src/codewhispererChat/controllers/chat/controller.ts

+        const fileExists = await fs.existsFile(filePath)
+        // Check if fileExists=false, If yes, return instead of showing broken diff experience.
+        if (!session.tempFilePath) {
+            return
+        }


does this notify the user?

No It wont notify the user for now, but good to have a Information notification message. I will add this to the TODO.

mr-lee · 2025-03-25T21:58:11Z

packages/core/src/codewhispererChat/clients/chat/v0/chat.ts

 export class ChatSession {
    private sessionId?: string
+    /**
+     * _listOfReadFiles = list of files read from the project to gather context before generating response.


why do you need to pass this around?

To count # of files read as per UX requirement.

Modifying Agentic chat UX for view code diff and accept/reject files

8d6467b

laileni-aws marked this pull request as ready for review March 25, 2025 00:53

laileni-aws requested a review from a team as a code owner March 25, 2025 00:53

laileni-aws changed the title ~~feat(amazonq): Agentic chat UX~~ feat(amazonq): Adding openDiff and AcceptDiff UX to the agentic chat. Mar 25, 2025

laileni-aws changed the title ~~feat(amazonq): Adding openDiff and AcceptDiff UX to the agentic chat.~~ feat(amazonq): Adding openDiff and acceptDiff UX to the agentic chat. Mar 25, 2025

ctlai95 reviewed Mar 25, 2025

View reviewed changes

laileni-aws added 2 commits March 25, 2025 13:39

Addressing comments

20b166e

Minor edits

f2d1467

laileni-aws requested a review from ctlai95 March 25, 2025 20:44

laileni-aws and others added 2 commits March 25, 2025 13:45

Merge branch 'feature/agentic-chat' into part1

8822a39

Minor text change

02478e3

ctlai95 reviewed Mar 25, 2025

View reviewed changes

mr-lee approved these changes Mar 25, 2025

View reviewed changes

mr-lee merged commit 8392a4c into aws:feature/agentic-chat Mar 25, 2025
15 of 17 checks passed

laileni-aws deleted the part1 branch April 21, 2025 03:31

	public get getFilePath(): string \| undefined {
	public get filePath(): string \| undefined {

	public get listOfReadFiles(): string[] {
	public get readFiles(): string[] {

feat(amazonq): Adding openDiff and acceptDiff UX to the agentic chat. #6846

feat(amazonq): Adding openDiff and acceptDiff UX to the agentic chat. #6846

Uh oh!

Conversation

laileni-aws commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

TODO:

Uh oh!

github-actions bot commented Mar 25, 2025

Uh oh!

ctlai95 commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ctlai95 Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ctlai95 Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laileni-aws Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laileni-aws Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

laileni-aws commented Mar 25, 2025 •

edited

Loading

ctlai95 Mar 25, 2025 •

edited

Loading

ctlai95 Mar 25, 2025 •

edited

Loading

laileni-aws Mar 25, 2025 •

edited

Loading

laileni-aws Mar 25, 2025 •

edited

Loading